Skip to content

Conversation

@leo-collins
Copy link
Contributor

@leo-collins leo-collins commented Oct 23, 2025

Description

I accidentally changed the API of Function.evaluate in #4543. This changes the shape of the output to how it was before. Now, if you pass a single point to Function.evaluate, then we remove the first axis of the result. For example, if

mesh = UnitSquareMesh(1, 1)
V = FunctionSpace(mesh, "CG", 1)
u = Function(V)

then u((0.5, 0.5)) returns a numpy array with shape (), and not an array with shape (1,). If u lives in a VectorFunctionSpace, then u((0.5, 0.5)) returns an array with shape(2,), and not an array with shape (1,2).

There were no tests looking for this before so I've added some.

@connorjward
Copy link
Contributor

This is a bug fix (and a critical one at that) so should go into release.

@pbrubeck
Copy link
Contributor

We should also fix calling a Function on a numpy.array

from firedrake import *
mesh = UnitSquareMesh(1, 1)
V = FunctionSpace(mesh, "CG", 1)
u = Function(V)
u(numpy.asarray([0.5, 0.5]))
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
Cell In[36], line 5
      3 V = FunctionSpace(mesh, "CG", 1)
      4 u = Function(V)
----> 5 u(numpy.asarray([0.5, 0.5]))

File /scratch/brubeckmarti/firedrake-venv/src/ufl/ufl/exproperators.py:345, in _call(self, arg, mapping, component)
    343 def _call(self, arg, mapping=None, component=()):
    344     """Take the restriction or evaluate depending on argument."""
--> 345     if arg in ("+", "-"):
    346         if mapping is not None:
    347             raise ValueError("Not expecting a mapping when taking restriction.")

ValueError: The truth value of an array with more than one element is ambiguous. Use a.any() or a.all()

We need something like
345 if isinstance(arg, str) and arg in ("+", "-"):

@pbrubeck
Copy link
Contributor

Ideally we would want Function.__call__ to behave like standard numpy operations if the points are numpy.arrays. This is why I insist that the shape must be preserved. FInAT and FIAT already respect this contract when tabulating finite elements on reference points.

@leo-collins leo-collins force-pushed the leo/fix-point-eval-api branch from 2df11b3 to b95a4a0 Compare October 29, 2025 15:28
@leo-collins leo-collins changed the base branch from main to release October 29, 2025 18:28
@leo-collins leo-collins force-pushed the leo/fix-point-eval-api branch 4 times, most recently from f94ae63 to af5cfcb Compare October 29, 2025 18:38
dham
dham previously approved these changes Nov 4, 2025
@connorjward
Copy link
Contributor

@leo-collins the recommendations from the meeting yesterday were:

TODO: release - revert to previous behaviour, main - do the sensible thing of allowing a user to pass a single position as a rank-1 vector rather than a rank-2 vector with a length 1 dimension.

I confess I got a bit confused by what we ended up deciding to do but if you agree with that please implement it here and poke me for a review when the tests are passing. I can then make a new release.

@leo-collins leo-collins force-pushed the leo/fix-point-eval-api branch from 5551e22 to 53eaec4 Compare November 10, 2025 11:32
@leo-collins
Copy link
Contributor Author

@connorjward This should be okay to merge (assuming the tests pass)

@connorjward
Copy link
Contributor

@leo-collins I am happy but please can you put more information in the PR description explaining all this?

@leo-collins
Copy link
Contributor Author

@connorjward Updated description.

@connorjward connorjward merged commit a33a610 into release Nov 10, 2025
14 checks passed
@connorjward connorjward deleted the leo/fix-point-eval-api branch November 10, 2025 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants